Repository navigation
feat(adapters/slack): add slackapi, a low-level Slack API package - #97
ethanndickson wants to merge 24 commits into
Conversation
…ntion helpers, and manifest types
…k, and slackapitest
…rects and check upload redirects DownloadFile sets the bearer token again on each redirect that passes the FileOrigins check, because http.Client drops it on a redirect to another host. UploadToURL checks the origin of every redirect with a per-call copy of the HTTP client and never adds a token. Origins drop the default port of the scheme, New warns about each dropped FileOrigins entry, and an empty file URL returns a clear error.
GetUploadURLExternal sends its form through callForm. The response type is now GetUploadURLExternalResponse, which matches CompleteUploadExternalResponse and does not stutter with its UploadURL field. UploadFile wraps step errors without a second slack: prefix.
… encode empty action elements SplitMarkdown tracks the backtick count of the opening fence: only a line of at least as many backticks and optional whitespace closes it, and the added closing fence repeats that count. A chunk no longer ends inside or right after an opening fence line, which left an empty code block. ActionsBlock encodes nil Elements as an empty array.
|
@codex review |
|
@codex security review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1cc5463fbd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1cc5463fbd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a4d7d5e183
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e887367dee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Maintainer review. I accept the direction. Below are a few targeted requests before this leaves draft. Direction. Putting the Slack protocol code into a low-level Requests:
After that, it's the normal gate: green CI, then one Generated with |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e887367dee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… and the app home manifest section User gets IsRestricted and IsUltraRestricted, and Message gets UserTeam, SourceTeam, Username, and BotProfile, so a bot can drop guests and users from another organization and name the app behind a bot message. ManifestFeatures gets AppHome, so a manifest can turn the Messages tab off.
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
The Slack adapter already does all the Slack protocol work, but you can only get at it through
chat.New. An application that runs its own chat engine, with its own state, dispatch, and routing (say, a Slack bot built into another server), can't use it. It also needs things the portable surface leaves out on purpose, like edits, deletes, reactions, and files.This PR adds
adapters/slack/slackapi, a low-level package for that: a typed Web API client with bounded retry, the request signature check, Events API parsing, file upload and host-restricted download, Block Kit types, and a Markdown splitter for Slack'smarkdownblock limit. There's alsoslackapitest, a fake Slack server for callers to test against. ADR 0016 has the reasoning.The adapter now uses
slackapifor its calls, retry, signature check, and history reads, so there's only one copy of each.RetryPolicyandRateLimitedare now aliases, so nothing breaks. The one behaviour change is that arate_limitederror in a 200 response now retries too, the same as a 429. The adapter still builds its own request bodies forauth.test,chat.postMessage,chat.postEphemeralandconversations.open, and I'll move those onto the typed methods in a follow-up.There's also an env-gated
TestLive, which I ran against my test workspace, and it passed. It showed that the 12,000 character limit applies to all themarkdownblocks in a message together, not to each block. It also showed that the adapter'sHistoryReaderwas broken on real Slack, since it sentconversations.historyandconversations.repliesas JSON:slackapisends those as form posts, which works, so the adapter now just reads history throughslackapi. The adapter tests' fake Slack server now rejects JSON for those methods too, like real Slack does.Most of the diff is tests. The first few commits split it up by area, and the rest are fixes on top.